netdev - Re: [PATCH net-next v2 2/2] net: maxlinear: Add support for MxL LGM SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <bb08014c-c85d-48cd-98d3-0a3fe8f890c6@kernel.org>
Date: Wed, 27 Aug 2025 14:33:50 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Jack Ping CHNG <jchng@...linear.com>, netdev@...r.kernel.org,
 devicetree@...r.kernel.org
Cc: davem@...emloft.net, andrew+netdev@...n.ch, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, yzhu@...linear.com, sureshnagaraj@...linear.com
Subject: Re: [PATCH net-next v2 2/2] net: maxlinear: Add support for MxL LGM
 SoC

On 26/08/2025 05:10, Jack Ping CHNG wrote:
> Introduce the build system integration and initial implementation for the

Nothing improved - do not describe how kernel works, builds. We all know
Kconfig.

Describe the driver and hardware.

> MaxLinear LGM SoC Ethernet driver. This patch adds Kconfig and Makefile

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95


...

> +
> +static int mxl_eth_probe(struct platform_device *pdev)
> +{
> +	struct mxl_eth_drvdata *drvdata;
> +	struct reset_control *rst;
> +	struct net_device *ndev;
> +	struct device_node *np;
> +	int ret, i;
> +
> +	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	drvdata->clks = devm_clk_get_enabled(&pdev->dev, "ethif");
> +	if (IS_ERR(drvdata->clks))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(drvdata->clks),

That's correct...

> +				     "failed to get/enable clock\n");
> +
> +	rst = devm_reset_control_get_optional(&pdev->dev, NULL);
> +	if (IS_ERR(rst)) {
> +		dev_err(&pdev->dev,
> +			"failed to get optional reset control: %ld\n",
> +			PTR_ERR(rst));


But here nothing improved.

Please look how ALL DRIVERS do it? What syntax they use?

Do your homework and really work on this driver. If I pointed issue on
clocks, YOU MUST fix it everywhere, not just clocks.


> +		ret = PTR_ERR(rst);
> +		goto err_cleanup;
> +	}
> +
> +	if (rst) {
> +		ret = reset_control_assert(rst);
> +		if (ret)
> +			goto err_cleanup;
> +
> +		udelay(1);
> +
> +		ret = reset_control_deassert(rst);
> +		if (ret)
> +			goto err_cleanup;
> +	}
> +
> +	platform_set_drvdata(pdev, drvdata);
> +
> +	i = 0;
> +	for_each_available_child_of_node(pdev->dev.of_node, np) {
> +		if (!of_device_is_compatible(np, "mxl,eth-mac"))
> +			continue;
> +
> +		ret = mxl_eth_create_ndev(pdev, np, &ndev);
> +		if (ret)
> +			goto err_cleanup;

You leak of node. Use scoped loop.

> +
> +		drvdata->ndevs[i++] = ndev;
> +		if (i >= MXL_NUM_PORT)
> +			break;
> +	}
> +
> +	return 0;
> +
> +err_cleanup:
> +	mxl_eth_cleanup(drvdata);
> +	return ret;
> +}
> +
> +static void mxl_eth_remove(struct platform_device *pdev)
> +{
> +	struct mxl_eth_drvdata *drvdata = platform_get_drvdata(pdev);
> +
> +	mxl_eth_cleanup(drvdata);
> +}
> +
> +/* Device Tree match table */
> +static const struct of_device_id mxl_eth_of_match[] = {
> +	{ .compatible = "mxl,lgm-eth" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mxl_eth_of_match);
> +
> +/* Platform driver struct */
> +static struct platform_driver mxl_eth_drv = {
> +	.probe    = mxl_eth_probe,
> +	.remove   = mxl_eth_remove,
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.of_match_table = mxl_eth_of_match,
> +	},
> +};
> +
> +module_platform_driver(mxl_eth_drv);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Ethernet driver for MxL SoC");


Best regards,
Krzysztof