Hi Simon,

On 14:17 Wed 21 Aug     , Simon Horman wrote:

On Tue, Aug 20, 2024 at 04:36:08PM +0200, Andrea della Porta wrote:

quoted

RaspberryPi RP1 is an MFD providing, among other peripherals, several
clock generators and PLLs that drives the sub-peripherals.
Add the driver to support the clock providers.

Signed-off-by: Andrea della Porta <andrea.porta@suse.com>

...

quoted

diff --git a/drivers/clk/clk-rp1.c b/drivers/clk/clk-rp1.c
new file mode 100644
index 000000000000..d18e711c0623
--- /dev/null
+++ b/drivers/clk/clk-rp1.c

@@ -0,0 +1,1655 @@
+// SPDX-License-Identifier: GPL

checkpatch says:

WARNING: 'SPDX-License-Identifier: GPL' is not supported in LICENSES/...

Alas, the system on which I executed checkpatch was missing git python module,
so spdxcheck.py wasn't working properly, sorry about that. Fixed in the next
release.

...

quoted

+static int rp1_clock_set_parent(struct clk_hw *hw, u8 index)
+{
+	struct rp1_clock *clock = container_of(hw, struct rp1_clock, hw);
+	struct rp1_clockman *clockman = clock->clockman;
+	const struct rp1_clock_data *data = clock->data;
+	u32 ctrl, sel;
+
+	spin_lock(&clockman->regs_lock);
+	ctrl = clockman_read(clockman, data->ctrl_reg);
+
+	if (index >= data->num_std_parents) {
+		/* This is an aux source request */
+		if (index >= data->num_std_parents + data->num_aux_parents)

It looks like &clockman->regs_lock needs to be unlocked here.

Flagged by Smatch, Sparse. and Coccinelle.

Ack.

Many thanks,
Andrea

quoted

+			return -EINVAL;
+
+		/* Select parent from aux list */
+		ctrl = set_register_field(ctrl, index - data->num_std_parents,
+					  CLK_CTRL_AUXSRC_MASK,
+					  CLK_CTRL_AUXSRC_SHIFT);
+		/* Set src to aux list */
+		ctrl = set_register_field(ctrl, AUX_SEL, data->clk_src_mask,
+					  CLK_CTRL_SRC_SHIFT);
+	} else {
+		ctrl = set_register_field(ctrl, index, data->clk_src_mask,
+					  CLK_CTRL_SRC_SHIFT);
+	}
+
+	clockman_write(clockman, data->ctrl_reg, ctrl);
+	spin_unlock(&clockman->regs_lock);
+
+	sel = rp1_clock_get_parent(hw);
+	WARN(sel != index, "(%s): Parent index req %u returned back %u\n",
+	     data->name, index, sel);
+
+	return 0;
+}

...