| Message ID | 20241028-upstream_s32cc_gmac-v4-14-03618f10e3e2@oss.nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for Synopsis DWMAC IP on NXP Automotive SoCs S32G2xx/S32G3xx/S32R45 | expand |
On Mon, Oct 28, 2024 at 09:24:56PM +0100, Jan Petrous (OSS) wrote: > + plat->init = s32_gmac_init; > + plat->exit = s32_gmac_exit; > + plat->fix_mac_speed = s32_fix_mac_speed; > + > + plat->bsp_priv = gmac; > + > + return stmmac_pltfr_probe(pdev, plat, &res); > +} > + > +static const struct of_device_id s32_dwmac_match[] = { > + { .compatible = "nxp,s32g2-dwmac" }, > + { .compatible = "nxp,s32g3-dwmac" }, > + { .compatible = "nxp,s32r-dwmac" }, Why do you need three same entries? Best regards, Krzysztof
> +#define GMAC_TX_RATE_125M 125000000 /* 125MHz */ > +#define GMAC_TX_RATE_25M 25000000 /* 25MHz */ > +#define GMAC_TX_RATE_2M5 2500000 /* 2.5MHz */ With the swap to the new helper, i think 25M and 2M5 are no longer needed. > +static int s32_gmac_init(struct platform_device *pdev, void *priv) > +{ > + struct s32_priv_data *gmac = priv; > + int ret; > + > + ret = clk_set_rate(gmac->tx_clk, GMAC_TX_RATE_125M); > + if (!ret) > + ret = clk_prepare_enable(gmac->tx_clk); > + > + if (ret) { > + dev_err(&pdev->dev, "Can't set tx clock\n"); > + return ret; > + } > + > + ret = clk_prepare_enable(gmac->rx_clk); > + if (ret) > + dev_dbg(&pdev->dev, "Can't set rx, clock source is disabled.\n"); > + else > + gmac->rx_clk_enabled = true; Why would this fail? And if it does fail, why is it not fatal? Maybe a comment here. > +static void s32_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode) > +{ > + struct s32_priv_data *gmac = priv; > + long tx_clk_rate; > + int ret; > + > + if (!gmac->rx_clk_enabled) { > + ret = clk_prepare_enable(gmac->rx_clk); > + if (ret) { > + dev_err(gmac->dev, "Can't set rx clock\n"); dev_err(), so is failing now fatal, but since this is a void function, you cannot report the error up the call stack? Andrew
On Tue, Oct 29, 2024 at 08:13:40AM +0100, Krzysztof Kozlowski wrote: > On Mon, Oct 28, 2024 at 09:24:56PM +0100, Jan Petrous (OSS) wrote: > > + plat->init = s32_gmac_init; > > + plat->exit = s32_gmac_exit; > > + plat->fix_mac_speed = s32_fix_mac_speed; > > + > > + plat->bsp_priv = gmac; > > + > > + return stmmac_pltfr_probe(pdev, plat, &res); > > +} > > + > > +static const struct of_device_id s32_dwmac_match[] = { > > + { .compatible = "nxp,s32g2-dwmac" }, > > + { .compatible = "nxp,s32g3-dwmac" }, > > + { .compatible = "nxp,s32r-dwmac" }, > > Why do you need three same entries? > We have three different SoCs and in v3 review you told me to return all back: https://patchwork.kernel.org/comment/26067257/ I'm not sure if we need s32g3 variant, it should depend on changes between s32g2 and s32g3, but s32r will definitely carry the interface max-speed value when SGMII support will be added. BR. /Jan
On 31/10/2024 15:43, Jan Petrous wrote: > On Tue, Oct 29, 2024 at 08:13:40AM +0100, Krzysztof Kozlowski wrote: >> On Mon, Oct 28, 2024 at 09:24:56PM +0100, Jan Petrous (OSS) wrote: >>> + plat->init = s32_gmac_init; >>> + plat->exit = s32_gmac_exit; >>> + plat->fix_mac_speed = s32_fix_mac_speed; >>> + >>> + plat->bsp_priv = gmac; >>> + >>> + return stmmac_pltfr_probe(pdev, plat, &res); >>> +} >>> + >>> +static const struct of_device_id s32_dwmac_match[] = { >>> + { .compatible = "nxp,s32g2-dwmac" }, >>> + { .compatible = "nxp,s32g3-dwmac" }, >>> + { .compatible = "nxp,s32r-dwmac" }, >> >> Why do you need three same entries? >> > > We have three different SoCs and in v3 review you told me > to return all back: > https://patchwork.kernel.org/comment/26067257/ It was about binding, not driver. I also asked there: use proper fallback and compatibility. Both comments of course affect your driver, but why choosing only first part? > > I'm not sure if we need s32g3 variant, it should depend on > changes between s32g2 and s32g3, but s32r will definitely > carry the interface max-speed value when SGMII support will > be added. > > BR. > /Jan Best regards, Krzysztof
On Thu, Oct 31, 2024 at 04:44:45PM +0100, Krzysztof Kozlowski wrote: > On 31/10/2024 15:43, Jan Petrous wrote: > > On Tue, Oct 29, 2024 at 08:13:40AM +0100, Krzysztof Kozlowski wrote: > >> On Mon, Oct 28, 2024 at 09:24:56PM +0100, Jan Petrous (OSS) wrote: > >>> + plat->init = s32_gmac_init; > >>> + plat->exit = s32_gmac_exit; > >>> + plat->fix_mac_speed = s32_fix_mac_speed; > >>> + > >>> + plat->bsp_priv = gmac; > >>> + > >>> + return stmmac_pltfr_probe(pdev, plat, &res); > >>> +} > >>> + > >>> +static const struct of_device_id s32_dwmac_match[] = { > >>> + { .compatible = "nxp,s32g2-dwmac" }, > >>> + { .compatible = "nxp,s32g3-dwmac" }, > >>> + { .compatible = "nxp,s32r-dwmac" }, > >> > >> Why do you need three same entries? > >> > > > > We have three different SoCs and in v3 review you told me > > to return all back: > > https://patchwork.kernel.org/comment/26067257/ > > It was about binding, not driver. > > I also asked there: use proper fallback and compatibility. Both comments > of course affect your driver, but why choosing only first part? > Does it mean I should remove first two (G2/G3) members from match array and use "nxp,s32r-dwmac" as fallback for G2/G3? And similarly change the bindings to: compatible: oneOf: - const: nxp,s32r-dwmac - items: - enum: - nxp,s32g2-dwmac - nxp,s32g3-dwmac - const: nxp,s32r-dwmac And add here, into the driver, those members back when some device specific feature will be needed? Am I understand your hints right? Thanks. /Jan
On Thu, Oct 31, 2024 at 06:16:46PM +0100, Jan Petrous wrote: > On Thu, Oct 31, 2024 at 04:44:45PM +0100, Krzysztof Kozlowski wrote: > > On 31/10/2024 15:43, Jan Petrous wrote: > > > On Tue, Oct 29, 2024 at 08:13:40AM +0100, Krzysztof Kozlowski wrote: > > >> On Mon, Oct 28, 2024 at 09:24:56PM +0100, Jan Petrous (OSS) wrote: > > >>> + plat->init = s32_gmac_init; > > >>> + plat->exit = s32_gmac_exit; > > >>> + plat->fix_mac_speed = s32_fix_mac_speed; > > >>> + > > >>> + plat->bsp_priv = gmac; > > >>> + > > >>> + return stmmac_pltfr_probe(pdev, plat, &res); > > >>> +} > > >>> + > > >>> +static const struct of_device_id s32_dwmac_match[] = { > > >>> + { .compatible = "nxp,s32g2-dwmac" }, > > >>> + { .compatible = "nxp,s32g3-dwmac" }, > > >>> + { .compatible = "nxp,s32r-dwmac" }, > > >> > > >> Why do you need three same entries? > > >> > > > > > > We have three different SoCs and in v3 review you told me > > > to return all back: > > > https://patchwork.kernel.org/comment/26067257/ > > > > It was about binding, not driver. > > > > I also asked there: use proper fallback and compatibility. Both comments > > of course affect your driver, but why choosing only first part? > > > > Does it mean I should remove first two (G2/G3) members from match array > and use "nxp,s32r-dwmac" as fallback for G2/G3? And similarly change > the bindings to: > > compatible: > oneOf: > - const: nxp,s32r-dwmac > - items: > - enum: > - nxp,s32g2-dwmac > - nxp,s32g3-dwmac > - const: nxp,s32r-dwmac > > And add here, into the driver, those members back when some device > specific feature will be needed? Am I understand your hints right? > Sorry, it's not correct. This way I'm not able to detect S32R which is the only one with higher speed. Then I could use the G2 as fallback I think, Ie.: compatible: oneOf: - const: nxp,s32g2-dwmac - items: - enum: - nxp,s32g3-dwmac - nxp,s32r-dwmac - const: nxp,s32g2-dwmac BR. /Jan
On 31/10/2024 18:16, Jan Petrous wrote: > On Thu, Oct 31, 2024 at 04:44:45PM +0100, Krzysztof Kozlowski wrote: >> On 31/10/2024 15:43, Jan Petrous wrote: >>> On Tue, Oct 29, 2024 at 08:13:40AM +0100, Krzysztof Kozlowski wrote: >>>> On Mon, Oct 28, 2024 at 09:24:56PM +0100, Jan Petrous (OSS) wrote: >>>>> + plat->init = s32_gmac_init; >>>>> + plat->exit = s32_gmac_exit; >>>>> + plat->fix_mac_speed = s32_fix_mac_speed; >>>>> + >>>>> + plat->bsp_priv = gmac; >>>>> + >>>>> + return stmmac_pltfr_probe(pdev, plat, &res); >>>>> +} >>>>> + >>>>> +static const struct of_device_id s32_dwmac_match[] = { >>>>> + { .compatible = "nxp,s32g2-dwmac" }, >>>>> + { .compatible = "nxp,s32g3-dwmac" }, >>>>> + { .compatible = "nxp,s32r-dwmac" }, >>>> >>>> Why do you need three same entries? >>>> >>> >>> We have three different SoCs and in v3 review you told me >>> to return all back: >>> https://patchwork.kernel.org/comment/26067257/ >> >> It was about binding, not driver. >> >> I also asked there: use proper fallback and compatibility. Both comments >> of course affect your driver, but why choosing only first part? >> > > Does it mean I should remove first two (G2/G3) members from match array > and use "nxp,s32r-dwmac" as fallback for G2/G3? And similarly change > the bindings to: > > compatible: > oneOf: > - const: nxp,s32r-dwmac > - items: > - enum: > - nxp,s32g2-dwmac > - nxp,s32g3-dwmac > - const: nxp,s32r-dwmac > > And add here, into the driver, those members back when some device > specific feature will be needed? Am I understand your hints right? Yes, assuming devices are compatible, but so far driver suggested they are. Best regards, Krzysztof
On 31/10/2024 18:24, Jan Petrous wrote: > On Thu, Oct 31, 2024 at 06:16:46PM +0100, Jan Petrous wrote: >> On Thu, Oct 31, 2024 at 04:44:45PM +0100, Krzysztof Kozlowski wrote: >>> On 31/10/2024 15:43, Jan Petrous wrote: >>>> On Tue, Oct 29, 2024 at 08:13:40AM +0100, Krzysztof Kozlowski wrote: >>>>> On Mon, Oct 28, 2024 at 09:24:56PM +0100, Jan Petrous (OSS) wrote: >>>>>> + plat->init = s32_gmac_init; >>>>>> + plat->exit = s32_gmac_exit; >>>>>> + plat->fix_mac_speed = s32_fix_mac_speed; >>>>>> + >>>>>> + plat->bsp_priv = gmac; >>>>>> + >>>>>> + return stmmac_pltfr_probe(pdev, plat, &res); >>>>>> +} >>>>>> + >>>>>> +static const struct of_device_id s32_dwmac_match[] = { >>>>>> + { .compatible = "nxp,s32g2-dwmac" }, >>>>>> + { .compatible = "nxp,s32g3-dwmac" }, >>>>>> + { .compatible = "nxp,s32r-dwmac" }, >>>>> >>>>> Why do you need three same entries? >>>>> >>>> >>>> We have three different SoCs and in v3 review you told me >>>> to return all back: >>>> https://patchwork.kernel.org/comment/26067257/ >>> >>> It was about binding, not driver. >>> >>> I also asked there: use proper fallback and compatibility. Both comments >>> of course affect your driver, but why choosing only first part? >>> >> >> Does it mean I should remove first two (G2/G3) members from match array >> and use "nxp,s32r-dwmac" as fallback for G2/G3? And similarly change >> the bindings to: >> >> compatible: >> oneOf: >> - const: nxp,s32r-dwmac >> - items: >> - enum: >> - nxp,s32g2-dwmac >> - nxp,s32g3-dwmac >> - const: nxp,s32r-dwmac >> >> And add here, into the driver, those members back when some device >> specific feature will be needed? Am I understand your hints right? >> > > Sorry, it's not correct. This way I'm not able to detect S32R which is > the only one with higher speed. > > Then I could use the G2 as fallback I think, Ie.: > > compatible: > oneOf: > - const: nxp,s32g2-dwmac > - items: > - enum: > - nxp,s32g3-dwmac > - nxp,s32r-dwmac > - const: nxp,s32g2-dwmac I don't understand. In both cases you can 'detect r', if by this you meant match and bind. I don't care which one is the fallback, but if one does not work it points to different issues with your code. Best regards, Krzysztof
On Tue, Oct 29, 2024 at 01:15:52PM +0100, Andrew Lunn wrote: > > +#define GMAC_TX_RATE_125M 125000000 /* 125MHz */ > > +#define GMAC_TX_RATE_25M 25000000 /* 25MHz */ > > +#define GMAC_TX_RATE_2M5 2500000 /* 2.5MHz */ > > With the swap to the new helper, i think 25M and 2M5 are no longer > needed. > Sure, I will fix it in v5. > > +static int s32_gmac_init(struct platform_device *pdev, void *priv) > > +{ > > + struct s32_priv_data *gmac = priv; > > + int ret; > > + > > + ret = clk_set_rate(gmac->tx_clk, GMAC_TX_RATE_125M); > > + if (!ret) > > + ret = clk_prepare_enable(gmac->tx_clk); > > + > > + if (ret) { > > + dev_err(&pdev->dev, "Can't set tx clock\n"); > > + return ret; > > + } > > + > > + ret = clk_prepare_enable(gmac->rx_clk); > > + if (ret) > > + dev_dbg(&pdev->dev, "Can't set rx, clock source is disabled.\n"); > > + else > > + gmac->rx_clk_enabled = true; > > Why would this fail? And if it does fail, why is it not fatal? Maybe a > comment here. > > > +static void s32_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode) > > +{ > > + struct s32_priv_data *gmac = priv; > > + long tx_clk_rate; > > + int ret; > > + > > + if (!gmac->rx_clk_enabled) { > > + ret = clk_prepare_enable(gmac->rx_clk); > > + if (ret) { > > + dev_err(gmac->dev, "Can't set rx clock\n"); > > dev_err(), so is failing now fatal, but since this is a void function, > you cannot report the error up the call stack? > I did a homework and checked the issue which was fixed by that 'lazy' rx clock enable procedure and got conslusion it is not needed anymore, as I was not able to reproduce the issue on the same board but with newer kernel version (6.6.32 versus 5.15.73). So I will simplify rx clock management in v5. BR. /Jan
On Fri, Nov 01, 2024 at 04:40:50PM +0100, Krzysztof Kozlowski wrote: > On 31/10/2024 18:24, Jan Petrous wrote: > > On Thu, Oct 31, 2024 at 06:16:46PM +0100, Jan Petrous wrote: > >> On Thu, Oct 31, 2024 at 04:44:45PM +0100, Krzysztof Kozlowski wrote: > >>> On 31/10/2024 15:43, Jan Petrous wrote: > >>>> On Tue, Oct 29, 2024 at 08:13:40AM +0100, Krzysztof Kozlowski wrote: > >>>>> On Mon, Oct 28, 2024 at 09:24:56PM +0100, Jan Petrous (OSS) wrote: > >>>>>> + plat->init = s32_gmac_init; > >>>>>> + plat->exit = s32_gmac_exit; > >>>>>> + plat->fix_mac_speed = s32_fix_mac_speed; > >>>>>> + > >>>>>> + plat->bsp_priv = gmac; > >>>>>> + > >>>>>> + return stmmac_pltfr_probe(pdev, plat, &res); > >>>>>> +} > >>>>>> + > >>>>>> +static const struct of_device_id s32_dwmac_match[] = { > >>>>>> + { .compatible = "nxp,s32g2-dwmac" }, > >>>>>> + { .compatible = "nxp,s32g3-dwmac" }, > >>>>>> + { .compatible = "nxp,s32r-dwmac" }, > >>>>> > >>>>> Why do you need three same entries? > >>>>> > >>>> > >>>> We have three different SoCs and in v3 review you told me > >>>> to return all back: > >>>> https://patchwork.kernel.org/comment/26067257/ > >>> > >>> It was about binding, not driver. > >>> > >>> I also asked there: use proper fallback and compatibility. Both comments > >>> of course affect your driver, but why choosing only first part? > >>> > >> > >> Does it mean I should remove first two (G2/G3) members from match array > >> and use "nxp,s32r-dwmac" as fallback for G2/G3? And similarly change > >> the bindings to: > >> > >> compatible: > >> oneOf: > >> - const: nxp,s32r-dwmac > >> - items: > >> - enum: > >> - nxp,s32g2-dwmac > >> - nxp,s32g3-dwmac > >> - const: nxp,s32r-dwmac > >> > >> And add here, into the driver, those members back when some device > >> specific feature will be needed? Am I understand your hints right? > >> > > > > Sorry, it's not correct. This way I'm not able to detect S32R which is > > the only one with higher speed. > > > > Then I could use the G2 as fallback I think, Ie.: > > > > compatible: > > oneOf: > > - const: nxp,s32g2-dwmac > > - items: > > - enum: > > - nxp,s32g3-dwmac > > - nxp,s32r-dwmac > > - const: nxp,s32g2-dwmac > > I don't understand. In both cases you can 'detect r', if by this you > meant match and bind. I don't care which one is the fallback, but if one > does not work it points to different issues with your code. > I see. I will use the last variant (with s32g2 as fallback) in v5. BR. /Jan
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig index 05cc07b8f48c..a6579377bedb 100644 --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig @@ -154,6 +154,18 @@ config DWMAC_RZN1 the stmmac device driver. This support can make use of a custom MII converter PCS device. +config DWMAC_S32 + tristate "NXP S32G/S32R GMAC support" + default ARCH_S32 + depends on OF && (ARCH_S32 || COMPILE_TEST) + help + Support for ethernet controller on NXP S32CC SOCs. + + This selects NXP SoC glue layer support for the stmmac + device driver. This driver is used for the S32CC series + SOCs GMAC ethernet controller, ie. S32G2xx, S32G3xx and + S32R45. + config DWMAC_SOCFPGA tristate "SOCFPGA dwmac support" default ARCH_INTEL_SOCFPGA diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile index c2f0e91f6bf8..1e87e2652c82 100644 --- a/drivers/net/ethernet/stmicro/stmmac/Makefile +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile @@ -22,6 +22,7 @@ obj-$(CONFIG_DWMAC_MESON) += dwmac-meson.o dwmac-meson8b.o obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o obj-$(CONFIG_DWMAC_RZN1) += dwmac-rzn1.o +obj-$(CONFIG_DWMAC_S32) += dwmac-s32.o obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c new file mode 100644 index 000000000000..fba221c37594 --- /dev/null +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c @@ -0,0 +1,226 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * NXP S32G/R GMAC glue layer + * + * Copyright 2019-2024 NXP + * + */ + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/device.h> +#include <linux/ethtool.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of_mdio.h> +#include <linux/of_address.h> +#include <linux/phy.h> +#include <linux/phylink.h> +#include <linux/platform_device.h> +#include <linux/stmmac.h> + +#include "stmmac_platform.h" + +#define GMAC_TX_RATE_125M 125000000 /* 125MHz */ +#define GMAC_TX_RATE_25M 25000000 /* 25MHz */ +#define GMAC_TX_RATE_2M5 2500000 /* 2.5MHz */ + +/* SoC PHY interface control register */ +#define PHY_INTF_SEL_MII 0x00 +#define PHY_INTF_SEL_SGMII 0x01 +#define PHY_INTF_SEL_RGMII 0x02 +#define PHY_INTF_SEL_RMII 0x08 + +struct s32_priv_data { + void __iomem *ioaddr; + void __iomem *ctrl_sts; + struct device *dev; + phy_interface_t intf_mode; + struct clk *tx_clk; + struct clk *rx_clk; + bool rx_clk_enabled; +}; + +static int s32_gmac_write_phy_intf_select(struct s32_priv_data *gmac) +{ + u32 intf_sel; + + switch (gmac->intf_mode) { + case PHY_INTERFACE_MODE_RGMII: + case PHY_INTERFACE_MODE_RGMII_ID: + case PHY_INTERFACE_MODE_RGMII_TXID: + case PHY_INTERFACE_MODE_RGMII_RXID: + intf_sel = PHY_INTF_SEL_RGMII; + break; + default: + dev_err(gmac->dev, "Unsupported PHY interface: %s\n", + phy_modes(gmac->intf_mode)); + return -EINVAL; + } + + writel(intf_sel, gmac->ctrl_sts); + + dev_dbg(gmac->dev, "PHY mode set to %s\n", phy_modes(gmac->intf_mode)); + + return 0; +} + +static int s32_gmac_init(struct platform_device *pdev, void *priv) +{ + struct s32_priv_data *gmac = priv; + int ret; + + ret = clk_set_rate(gmac->tx_clk, GMAC_TX_RATE_125M); + if (!ret) + ret = clk_prepare_enable(gmac->tx_clk); + + if (ret) { + dev_err(&pdev->dev, "Can't set tx clock\n"); + return ret; + } + + ret = clk_prepare_enable(gmac->rx_clk); + if (ret) + dev_dbg(&pdev->dev, "Can't set rx, clock source is disabled.\n"); + else + gmac->rx_clk_enabled = true; + + ret = s32_gmac_write_phy_intf_select(gmac); + if (ret) { + clk_disable_unprepare(gmac->tx_clk); + if (gmac->rx_clk_enabled) { + clk_disable_unprepare(gmac->rx_clk); + gmac->rx_clk_enabled = false; + } + + dev_err(&pdev->dev, "Can't set PHY interface mode\n"); + return ret; + } + + return 0; +} + +static void s32_gmac_exit(struct platform_device *pdev, void *priv) +{ + struct s32_priv_data *gmac = priv; + + clk_disable_unprepare(gmac->tx_clk); + + if (gmac->rx_clk_enabled) { + clk_disable_unprepare(gmac->rx_clk); + gmac->rx_clk_enabled = false; + } +} + +static void s32_fix_mac_speed(void *priv, unsigned int speed, unsigned int mode) +{ + struct s32_priv_data *gmac = priv; + long tx_clk_rate; + int ret; + + if (!gmac->rx_clk_enabled) { + ret = clk_prepare_enable(gmac->rx_clk); + if (ret) { + dev_err(gmac->dev, "Can't set rx clock\n"); + return; + } + dev_dbg(gmac->dev, "rx clock enabled\n"); + gmac->rx_clk_enabled = true; + } + + tx_clk_rate = rgmii_clock(speed); + if (tx_clk_rate < 0) { + dev_err(gmac->dev, "Unsupported/Invalid speed: %d\n", speed); + return; + } + + dev_dbg(gmac->dev, "Set tx clock to %ld Hz\n", tx_clk_rate); + ret = clk_set_rate(gmac->tx_clk, tx_clk_rate); + if (ret) + dev_err(gmac->dev, "Can't set tx clock\n"); +} + +static int s32_dwmac_probe(struct platform_device *pdev) +{ + struct plat_stmmacenet_data *plat; + struct device *dev = &pdev->dev; + struct s32_priv_data *gmac; + struct stmmac_resources res; + int ret; + + gmac = devm_kzalloc(&pdev->dev, sizeof(*gmac), GFP_KERNEL); + if (!gmac) + return -ENOMEM; + + gmac->dev = &pdev->dev; + + ret = stmmac_get_platform_resources(pdev, &res); + if (ret) + return dev_err_probe(dev, ret, + "Failed to get platform resources\n"); + + plat = devm_stmmac_probe_config_dt(pdev, res.mac); + if (IS_ERR(plat)) + return dev_err_probe(dev, PTR_ERR(plat), + "dt configuration failed\n"); + + /* PHY interface mode control reg */ + gmac->ctrl_sts = devm_platform_get_and_ioremap_resource(pdev, 1, NULL); + if (IS_ERR(gmac->ctrl_sts)) + return dev_err_probe(dev, PTR_ERR(gmac->ctrl_sts), + "S32CC config region is missing\n"); + + /* tx clock */ + gmac->tx_clk = devm_clk_get(&pdev->dev, "tx"); + if (IS_ERR(gmac->tx_clk)) + return dev_err_probe(dev, PTR_ERR(gmac->tx_clk), + "tx clock not found\n"); + + /* rx clock */ + gmac->rx_clk = devm_clk_get(&pdev->dev, "rx"); + if (IS_ERR(gmac->rx_clk)) + return dev_err_probe(dev, PTR_ERR(gmac->rx_clk), + "rx clock not found\n"); + + gmac->intf_mode = plat->phy_interface; + gmac->ioaddr = res.addr; + + /* S32CC core feature set */ + plat->has_gmac4 = true; + plat->pmt = 1; + plat->flags |= STMMAC_FLAG_SPH_DISABLE; + plat->rx_fifo_size = 20480; + plat->tx_fifo_size = 20480; + + plat->init = s32_gmac_init; + plat->exit = s32_gmac_exit; + plat->fix_mac_speed = s32_fix_mac_speed; + + plat->bsp_priv = gmac; + + return stmmac_pltfr_probe(pdev, plat, &res); +} + +static const struct of_device_id s32_dwmac_match[] = { + { .compatible = "nxp,s32g2-dwmac" }, + { .compatible = "nxp,s32g3-dwmac" }, + { .compatible = "nxp,s32r-dwmac" }, + { } +}; +MODULE_DEVICE_TABLE(of, s32_dwmac_match); + +static struct platform_driver s32_dwmac_driver = { + .probe = s32_dwmac_probe, + .remove = stmmac_pltfr_remove, + .driver = { + .name = "s32-dwmac", + .pm = &stmmac_pltfr_pm_ops, + .of_match_table = s32_dwmac_match, + }, +}; +module_platform_driver(s32_dwmac_driver); + +MODULE_AUTHOR("Jan Petrous (OSS) <jan.petrous@oss.nxp.com>"); +MODULE_DESCRIPTION("NXP S32G/R common chassis GMAC driver"); +MODULE_LICENSE("GPL"); +