[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2379380.ElGaqSPkdT@fw-rgant>
Date: Mon, 10 Feb 2025 12:01:56 +0100
From: Romain Gantois <romain.gantois@...tlin.com>
To: Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Chen Wang <unicorn_wang@...look.com>,
Inochi Amaoto <inochiama@...look.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Richard Cochran <richardcochran@...il.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Emil Renner Berthing <emil.renner.berthing@...onical.com>,
Jisheng Zhang <jszhang@...nel.org>,
"Jan Petrous (OSS)" <jan.petrous@....nxp.com>,
Clément Léger <clement.leger@...tlin.com>,
Simon Horman <horms@...nel.org>, Furong Xu <0x1207@...il.com>,
Serge Semin <fancer.lancer@...il.com>, Lothar Rubusch <l.rubusch@...il.com>,
Suraj Jaiswal <quic_jsuraj@...cinc.com>,
Joe Hattori <joe@...is.s.u-tokyo.ac.jp>,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
Giuseppe Cavallaro <peppe.cavallaro@...com>,
Jose Abreu <joabreu@...opsys.com>, Inochi Amaoto <inochiama@...il.com>
Cc: Inochi Amaoto <inochiama@...il.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-riscv@...ts.infradead.org,
Yixun Lan <dlan@...too.org>, Longbin Li <looong.bin@...il.com>
Subject:
Re: [PATCH net-next v4 3/3] net: stmmac: Add glue layer for Sophgo SG2044 SoC
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@...il.com>
> + */
> +
> +#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@...il.com>");
> +MODULE_DESCRIPTION("Sophgo DWMAC platform driver");
> +MODULE_LICENSE("GPL");
Thanks,
--
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists