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
| ||
|
Message-ID: <155320bf-94d0-4e30-9283-d8ad178a323f@loongson.cn> Date: Sun, 10 Dec 2023 09:46:16 +0800 From: Yanteng Si <siyanteng@...ngson.cn> To: Andrew Lunn <andrew@...n.ch> Cc: hkallweit1@...il.com, peppe.cavallaro@...com, alexandre.torgue@...s.st.com, joabreu@...opsys.com, fancer.lancer@...il.com, Jose.Abreu@...opsys.com, chenhuacai@...ngson.cn, linux@...linux.org.uk, dongbiao@...ngson.cn, guyinggang@...ngson.cn, netdev@...r.kernel.org, loongarch@...ts.linux.dev, chris.chenfeiyang@...il.com Subject: Re: [PATCH v5 4/9] net: stmmac: dwmac-loongson: Refactor code for loongson_dwmac_probe() 在 2023/11/12 04:19, Andrew Lunn 写道: > On Fri, Nov 10, 2023 at 05:25:43PM +0800, Yanteng Si wrote: >> Add a setup() function to initialize data, and simplify code for >> loongson_dwmac_probe(). > This does not look like a refactoring patch. Such patches just move > code around, but otherwise leave the code alone. There are real > changes in here. > >> - if (!of_device_is_compatible(np, "loongson, pci-gmac")) { >> - pr_info("dwmac_loongson_pci: Incompatible OF node\n"); >> - return -ENODEV; >> - } >> - > This just disappears. Why is it no longer needed? > > >> plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL); >> if (!plat) >> return -ENOMEM; >> >> + plat->mdio_bus_data = devm_kzalloc(&pdev->dev, >> + sizeof(*plat->mdio_bus_data), >> + GFP_KERNEL); >> + if (!plat->mdio_bus_data) >> + return -ENOMEM; >> + >> + plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg), >> + GFP_KERNEL); >> + if (!plat->dma_cfg) >> + return -ENOMEM; >> + >> plat->mdio_node = of_get_child_by_name(np, "mdio"); >> if (plat->mdio_node) { >> dev_info(&pdev->dev, "Found MDIO subnode\n"); >> - >> - plat->mdio_bus_data = devm_kzalloc(&pdev->dev, >> - sizeof(*plat->mdio_bus_data), >> - GFP_KERNEL); >> - if (!plat->mdio_bus_data) { >> - ret = -ENOMEM; >> - goto err_put_node; >> - } > MDIO was conditional, but now is mandatory. Why? > >> if (ret) { >> - dev_err(&pdev->dev, "%s: ERROR: failed to enable device\n", __func__); >> + dev_err(&pdev->dev, "%s: ERROR: failed to enable device\n", >> + __func__); > Changes like this are a distraction. The reviewer is trying to > understand what has changed and why. If you want to make white space > changes, please do it in a patch of its own. > > Please break this patch up into lots of smaller parts, each with a > good commit messaged explaining what is going on, and importantly, > why. OK,Iwillgiveitatry. Thanks, Yanteng > > Andrew
Powered by blists - more mailing lists