Found a minor bug in drivers/video/ds90ub947-q1.c

Robotics & Edge Computing Jetson & Embedded Systems Jetson TX2
1

This does not affect driver functionality, it affects only the creation of debugfs file with registers images. The return value check from “ret = debugfs_create_file(…)” causes an error return when it is successful and vice versa… Less common - if debugfs_create_dir returns an error code instead of pointer it will interpreted as success.

Fixes below:

diff --git a/drivers/video/ds90ub947-q1.c b/drivers/video/ds90ub947-q1.c
index d2270e3..e56ad3a 100644
--- a/drivers/video/ds90ub947-q1.c
+++ b/drivers/video/ds90ub947-q1.c
@@ -212,12 +212,12 @@ static void ds90ub947_panel_create_debugfs(struct ds90ub947_data *data)
 	struct dentry *ret;
 
 	data->debugdir = debugfs_create_dir(DEV_NAME, NULL);
-	if (!data->debugdir)
+	if (IS_ERR_OR_NULL(data->debugdir))
 		goto err;
 
 	ret = debugfs_create_file("init_regs", S_IRUGO, data->debugdir, data,
 		&init_regs_fops);
-	if (ret)
+	if (IS_ERR_OR_NULL(ret))
 		goto err;
 
 	return;
2

Many thanks for this sharing!

3

Found another minor bug (or is it a feature?): if the device tree for a node using this driver does not have an “init-regs” value the parsing is ended and does not parse anything else (except the “ti,enable-gpio” entry which is parsed before “init-regs”).

diff --git a/drivers/video/ds90ub947-q1.c b/drivers/video/ds90ub947-q1.c
index e56ad3a..91c723a 100644
--- a/drivers/video/ds90ub947-q1.c
+++ b/drivers/video/ds90ub947-q1.c
@@ -256,24 +256,24 @@ static int of_ds90ub947_parse_pdata(struct i2c_client *client,
 	of_property_for_each_u32(np, "init-regs", prop, p, u)
 		count++;
 
-	if (!count)
-		return 0;
+	if (count) {
 
-	if ((count % 3) != 0) {
-		dev_err(&data->client->dev, "invalid \"init-regs\" data\n");
-		return -EINVAL;
-	}
+		if ((count % 3) != 0) {
+			dev_err(&data->client->dev, "invalid \"init-regs\" data\n");
+			return -EINVAL;
+		}
 
-	init_regs = devm_kzalloc(&client->dev, count * sizeof(u), GFP_KERNEL);
-	if (!init_regs) {
-		dev_err(&client->dev, "Failed to allocate init_regs\n");
-		return -ENOMEM;
-	}
+		init_regs = devm_kzalloc(&client->dev, count * sizeof(u), GFP_KERNEL);
+		if (!init_regs) {
+			dev_err(&client->dev, "Failed to allocate init_regs\n");
+			return -ENOMEM;
+		}
 
-	of_property_for_each_u32(np, "init-regs", prop, p, u)
-		init_regs[i++] = u & 0xff;
+		of_property_for_each_u32(np, "init-regs", prop, p, u)
+			init_regs[i++] = u & 0xff;
 
-	data->init_regs = init_regs;
+		data->init_regs = init_regs;
+	}
 	data->n_init_regs = count;
 
 	if (!of_property_read_u32(np, "ti,enable-bchnl-i2c", &temp))