On 8/5/25 17:32, Andrew Lunn wrote:

On Tue, Aug 05, 2025 at 11:34:55AM -0400, Sean Anderson wrote:

quoted

Rearrange the lifetime functions (probe, remove, etc.) in preparation
for the next commit. No functional change intended.

There is a lot going on in this patch. Can it be broken up a bit more?

The phase "No functional change intended" generally means, its the
same code, just in a different place in the files. This is not true of
this patch.

Sorry, at one point that was true and then I made a some edits. I will
update the commit message.

quoted

+struct axienet_common {
+	struct platform_device *pdev;
+
+	struct clk *axi_clk;
+
+	struct mutex reset_lock;

quoted

 static inline void axienet_lock_mii(struct axienet_local *lp)
 {
-	if (lp->mii_bus)
-		mutex_lock(&lp->mii_bus->mdio_lock);
+	mutex_lock(&lp->cp->reset_lock);

This lock is different to the bus lock. This is definitely not a "no
functional change".

Please make this lock change a patch of its own, with a good commit
message which considers the consequences of this change of lock.

OK

quoted

 		if (!np) {
-			dev_err(dev, "pcs-handle (preferred) or phy-handle required for 1000BaseX/SGMII\n");
-			ret = -EINVAL;
-			goto cleanup_mdio;
+			dev_err(dev,
+				"pcs-handle (preferred) or phy-handle required for 1000BaseX/SGMII\n");
+			return -EINVAL;

That looks like a whitespace change. This is a "No functional change
intended" sort of patch. You can collect all such whitespace changes
into one patch.

The main purpose of that hunk is to remove the `goto cleanup_mdio`. The
dev_err change is just because I was "in the area".

quoted

 		}
 		lp->pcs_phy = of_mdio_find_device(np);
-		np1 = of_find_node_by_name(NULL, "lpu");
+		np1 = of_find_node_by_name(NULL, "cpu");

Interesting. Maybe you should review your own patches.

Will do.

--Sean