Hello Inochi,
On dimanche 9 février 2025 02:30:52 heure normale d’Europe centrale Inochi
Amaoto wrote:
> Adds Sophgo dwmac driver support on the Sophgo SG2044 SoC.
...
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sophgo.c
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Sophgo DWMAC platform driver
> + *
> + * Copyright (C) 2024 Inochi Amaoto <inochiama@xxxxxxxxx>
> + */
> +
> +#include <linux/bits.h>
It doesn't look like this include is used, could you please remove it?
> +#include <linux/mod_devicetable.h>
> +#include <linux/phy.h>
> +#include <linux/platform_device.h>
> +
> +#include "stmmac_platform.h"
> +
> +struct sophgo_dwmac {
> + struct device *dev;
> + struct clk *clk_tx;
> +};
> +
> +static void sophgo_dwmac_fix_mac_speed(void *priv, unsigned int speed,
> unsigned int mode) +{
> + struct sophgo_dwmac *dwmac = priv;
> + long rate;
> + int ret;
> +
> + rate = rgmii_clock(speed);
> + if (rate < 0) {
> + dev_err(dwmac->dev, "invalid speed %u\n", speed);
> + return;
> + }
> +
> + ret = clk_set_rate(dwmac->clk_tx, rate);
> + if (ret)
> + dev_err(dwmac->dev, "failed to set tx rate %lu: %pe\n",
nit: shouldn't this be "%ld"?
> + rate, ERR_PTR(ret));
> +}
> +
> +static int sophgo_sg2044_dwmac_init(struct platform_device *pdev,
> + struct plat_stmmacenet_data *plat_dat,
> + struct stmmac_resources *stmmac_res)
> +{
> + struct sophgo_dwmac *dwmac;
> +
> + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> + if (!dwmac)
> + return -ENOMEM;
> +
> + dwmac->clk_tx = devm_clk_get_enabled(&pdev->dev, "tx");
> + if (IS_ERR(dwmac->clk_tx))
> + return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
> + "failed to get tx clock\n");
> +
> + dwmac->dev = &pdev->dev;
> + plat_dat->bsp_priv = dwmac;
> + plat_dat->flags |= STMMAC_FLAG_SPH_DISABLE;
> + plat_dat->fix_mac_speed = sophgo_dwmac_fix_mac_speed;
> + plat_dat->multicast_filter_bins = 0;
> + plat_dat->unicast_filter_entries = 1;
> +
> + return 0;
> +}
> +
> +static int sophgo_dwmac_probe(struct platform_device *pdev)
> +{
> + struct plat_stmmacenet_data *plat_dat;
> + struct stmmac_resources stmmac_res;
nit: I think adding "struct device *dev = &pdev->dev;" here would
be better than repeating "&pdev->dev" later on.
> + int ret;
> +
> + ret = stmmac_get_platform_resources(pdev, &stmmac_res);
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to get resources\n");
This error message is a bit too vague, maybe replace it with "failed to get
platform resources"?
> +
> + plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
> + if (IS_ERR(plat_dat))
> + return dev_err_probe(&pdev->dev, PTR_ERR(plat_dat),
> + "dt configuration failed\n");
This error message is a bit misleading IMO, I would replace it with
something like "failed to parse device-tree parameters".
> +
> + ret = sophgo_sg2044_dwmac_init(pdev, plat_dat, &stmmac_res);
> + if (ret)
> + return ret;
> +
> + return stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> +}
> +
> +static const struct of_device_id sophgo_dwmac_match[] = {
> + { .compatible = "sophgo,sg2044-dwmac" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sophgo_dwmac_match);
> +
> +static struct platform_driver sophgo_dwmac_driver = {
> + .probe = sophgo_dwmac_probe,
> + .remove = stmmac_pltfr_remove,
> + .driver = {
> + .name = "sophgo-dwmac",
> + .pm = &stmmac_pltfr_pm_ops,
> + .of_match_table = sophgo_dwmac_match,
> + },
> +};
> +module_platform_driver(sophgo_dwmac_driver);
> +
> +MODULE_AUTHOR("Inochi Amaoto <inochiama@xxxxxxxxx>");
> +MODULE_DESCRIPTION("Sophgo DWMAC platform driver");
> +MODULE_LICENSE("GPL");
Thanks,
--
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Attachment:
signature.asc
Description: This is a digitally signed message part.
