On Thu, Sep 29, 2016 at 12:34:39PM -0400, Olimpiu Dejeu wrote:

Resubmition of arcxcnn backliught driver adding definitions of the
 internal registers of the chip. 

s/backliught/backlight/

[...]

+#ifdef CONFIG_OF
+static int arcxcnn_parse_dt(struct arcxcnn *lp)
+{
+	struct device *dev = lp->dev;
+	struct device_node *node = dev->of_node;
+	u32 prog_length, prog_val, num_entry, sources[6];
+	int ret;
+
+	if (!node) {
+		dev_err(dev, "no platform data.\n");
+		return -EINVAL;
+	}
+	of_property_read_string(node, "label", &lp->pdata->name);
+	of_property_read_u32(node, "default-brightness", &prog_val);
+	lp->pdata->initial_brightness = prog_val;

You must check the return value of the of_ accessors. If there's no
default-brightness property in the DT, prog_val will be uninitialised here.

+	of_property_read_u32(node, "pwm-period", &lp->pdata->period_ns);

This was not in the binding document.

+	/* Fill program from platform data if defined
+	*/
+	lp->pdata->prog_entries = 0;
+	prog_length = of_get_child_count(node);
+	if (prog_length > 0) {
+		struct device_node *child;
+		u32 i = 0;
+
+		for_each_child_of_node(node, child) {
+			if (i >= ARCXCNN_MAX_PROGENTRIES)
+				break;
+			of_property_read_u32(child, "reg", &prog_val);
+			lp->pdata->prog_addr[i] = (u8)prog_val;
+			of_property_read_u32(child, "val", &prog_val);
+			lp->pdata->prog_data[i] = (u8)prog_val;
+			i++;
+		}
+		lp->pdata->prog_entries = i;
+	}

This was also not documented.

What is this? What is it used for? Why is it in the DT?

+	/* playback program now that its all installed */
+	for (i = 0; i < lp->pdata->prog_entries; i++) {
+		arcxcnn_write_byte(lp, lp->pdata->prog_addr[i],
+				lp->pdata->prog_data[i]);
+		/*
+		printk(KERN_ALERT "%02X, %02X\n",
+			lp->pdata->prog_addr[i], lp->pdata->prog_data[i]);
+		*/
+	}

Why is it necessary to have an arbitrary register programming sequence? What
exactly varies between parts?

I don't think this is the right way to solve your problem.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html