Thread (20 messages) 20 messages, 3 authors, 14d ago

Re: [PATCH v3 07/10] mfd: wm831x: support software nodes for DCDC regulators

From: sashiko-bot@kernel.org
Date: 2026-07-12 02:20:22
Also in: linux-devicetree

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The patch dynamically duplicates and modifies an `mfd_cell` array to inject software nodes, directly violating MFD subsystem guidelines.
- [Low] The commit message description starts with a lowercase letter, violating MFD subsystem formatting rules.
--
--- Patch [7]: [PATCH v3 07/10] mfd: wm831x: support software nodes for DCDC regulators ---
commit 3d69bbb3a9fcf1555157438b7a85ed2a2744e94b
Author: Dmitry Torokhov [off-list ref]

mfd: wm831x: support software nodes for DCDC regulators

This commit allows passing software nodes to wm831x DCDC regulators via PMIC
platform data. It adds dcdc_swnodes to the PMIC platform data and dynamically
assigns these software nodes to the buckv cells during device addition.
mfd: wm831x: support software nodes for DCDC regulators
[Severity: Low]
This isn't a bug, but should the description after the subsystem prefix be
capitalized? The MFD subsystem conventions generally expect:
"mfd: wm831x: Support software nodes for DCDC regulators".
quoted hunk ↗ jump to hunk
diff --git a/drivers/mfd/wm831x-core.c b/drivers/mfd/wm831x-core.c
--- a/drivers/mfd/wm831x-core.c
+++ b/drivers/mfd/wm831x-core.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1449,6 +1450,32 @@ const struct of_device_id wm831x_of_match[] = {
 };
 EXPORT_SYMBOL_GPL(wm831x_of_match);
 
+static int wm831x_add_devices(struct wm831x *wm831x, int wm831x_num,
+			      const struct mfd_cell *cells, int num_cells)
+{
+	struct wm831x_pdata *pdata = &wm831x->pdata;
+	int i;
+
+	struct mfd_cell *local_cells __free(kfree) =
+		kmemdup_array(cells, num_cells, sizeof(*cells), GFP_KERNEL);
[Severity: Medium]
Does this code dynamically duplicate the mfd_cell array to amend it?

The MFD subsystem guidelines advise that mfd_cell arrays should remain static
references, and we should avoid creating local copies for dynamic amendments.
Is there an alternative way to pass the software nodes without allocating and
modifying a duplicated cell array?
+	if (!local_cells)
+		return -ENOMEM;
+
+	for (i = 0; i < num_cells; i++) {
+		struct mfd_cell *cell = &local_cells[i];
+
+		if (strcmp(cell->name, "wm831x-buckv") == 0) {
+			int id = cell->id - 1;
+
+			if (id >= 0 && id < WM831X_MAX_DCDC)
+				cell->swnode = pdata->dcdc_swnodes[id];
+		}
+	}
+
+	return mfd_add_devices(wm831x->dev, wm831x_num, local_cells, num_cells,
+			       NULL, 0, NULL);
+}
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711-samsung-kp-v3-0-b2fcaba77aff@gmail.com?part=7
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help