Re: clk mess on omap4460 with mpu clock

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Mon, Sep 23, 2024 at 02:04:47PM +0200, Andreas Kemnade wrote:
> The main question what bothers me is whether we have
> some real problems behind it. The warning message is just an indicator
> of something odd which was already odd before the message was
> introduced.

Indeed.

> I have seen something working with some u-boot and some other not,
> so things might not get properly initialized... 
> 
> So the way forward is to check whether that registration is really
> needed at:
> https://elixir.bootlin.com/linux/v6.11/source/drivers/bus/ti-sysc.c#L2380
> If yes, then
> a) increade the size of the name in the clk subsystem or
> b) workaround like
> https://elixir.bootlin.com/linux/v6.11/source/drivers/bus/ti-sysc.c#L353

Or we make the arrays larger - at the moment, the struct is a nice round
64 bytes in 32-bit systems - 6 pointers (24 bytes) plus 24 plus 16 = 64.
For 64-bit systems, this is 88 bytes.

An alternative approach may be this (untested, not even compile tested):

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 2f83fb97c6fb..222f0ccf9fc0 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -149,8 +149,7 @@ void clkdev_add_table(struct clk_lookup *cl, size_t num)
 
 struct clk_lookup_alloc {
 	struct clk_lookup cl;
-	char	dev_id[MAX_DEV_ID];
-	char	con_id[MAX_CON_ID];
+	char	strings[0];
 };
 
 static struct clk_lookup * __ref
@@ -158,60 +157,36 @@ vclkdev_alloc(struct clk_hw *hw, const char *con_id, const char *dev_fmt,
 	va_list ap)
 {
 	struct clk_lookup_alloc *cla;
-	struct va_format vaf;
-	const char *failure;
 	va_list ap_copy;
-	size_t max_size;
-	ssize_t res;
+	size_t size;
+	char *strp;
 
-	cla = kzalloc(sizeof(*cla), GFP_KERNEL);
+	size = sizeof(*cla);
+	if (con_id)
+		size += 1 + strlen(con_id);
+	if (dev_fmt) {
+		va_copy(ap_copy, ap);
+		size += 1 + vsprintf(NULL, dev_fmt, ap_copy);
+		va_end(ap_copy);
+	}
+
+	cla = kzalloc(size, GFP_KERNEL);
 	if (!cla)
 		return NULL;
 
-	va_copy(ap_copy, ap);
-
 	cla->cl.clk_hw = hw;
+	strp = cla->strings;
 	if (con_id) {
-		res = strscpy(cla->con_id, con_id, sizeof(cla->con_id));
-		if (res < 0) {
-			max_size = sizeof(cla->con_id);
-			failure = "connection";
-			goto fail;
-		}
-		cla->cl.con_id = cla->con_id;
+		strcpy(strp, con_id);
+		cla->cl.con_id = strp;
+		strp += 1 + strlen(con_id);
 	}
 
 	if (dev_fmt) {
-		res = vsnprintf(cla->dev_id, sizeof(cla->dev_id), dev_fmt, ap);
-		if (res >= sizeof(cla->dev_id)) {
-			max_size = sizeof(cla->dev_id);
-			failure = "device";
-			goto fail;
-		}
-		cla->cl.dev_id = cla->dev_id;
+		vsprintf(strp, dev_fmt, ap);
+		cla->cl.dev_id = strp;
 	}
 
-	va_end(ap_copy);
-
-	return &cla->cl;
-
-fail:
-	if (dev_fmt)
-		vaf.fmt = dev_fmt;
-	else
-		vaf.fmt = "null-device";
-	vaf.va = &ap_copy;
-	pr_err("%pV:%s: %s ID is greater than %zu\n",
-	       &vaf, con_id, failure, max_size);
-	va_end(ap_copy);
-
-	/*
-	 * Don't fail in this case, but as the entry won't ever match just
-	 * fill it with something that also won't match.
-	 */
-	strscpy(cla->con_id, "bad", sizeof(cla->con_id));
-	strscpy(cla->dev_id, "bad", sizeof(cla->dev_id));
-
 	return &cla->cl;
 }
 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!