On Wed, Aug 17, 2016 at 03:06:39PM -0400, Olimpiu Dejeu wrote:

This is the driver for the Arctic Sand Technologies Inc. ARCXCNN LED
backlight IC family.

Signed-off-by: Olimpiu Dejeu <redacted>
---
 drivers/video/backlight/Kconfig      |   7 +
 drivers/video/backlight/Makefile     |   1 +
 drivers/video/backlight/arcxcnn_bl.c | 499 +++++++++++++++++++++++++++++++++++
 include/linux/i2c/arcxcnn.h          |  41 +++
 4 files changed, 547 insertions(+)
 create mode 100644 drivers/video/backlight/arcxcnn_bl.c
 create mode 100644 include/linux/i2c/arcxcnn.h

This is a one-patch series with no binding, yet the code below adds a new
compatible string ("arc,arcxcnn"), and DT parsing code assuming
uncommon, non-trivial properties.

You must write a device tree binding document for this.

Please see Documentation/devicetree/bindings/submitting-patches.txt.

+static int arcxcnn_parse_dt(struct arcxcnn *lp)
+{
+	struct device *dev = lp->dev;
+	struct device_node *node = dev->of_node;
+	u32 prog_length;
+
+	if (!node) {
+		dev_err(dev, "no platform data\n");
+		return -EINVAL;
+	}
+
+	of_property_read_string(node, "bl-name", &lp->pdata->name);
+	of_property_read_u16(node, "init-brt", &lp->pdata->initial_brightness);
+	of_property_read_u32(node, "pwm-period", &lp->pdata->period_ns);
+
+	if (lp->pdata->initial_brightness > MAX_BRIGHTNESS)
+		lp->pdata->initial_brightness = MAX_BRIGHTNESS;
+
+	/* 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_u8(child, "prg-addr", &lp->pdata->prog_addr[i]);
+			of_property_read_u8(child, "prg-data", &lp->pdata->prog_data[i]);

Don't bother using u8. The DTB is padded to u32 boundaries anyway, so this
just adds an additional burden on DT authors to use /bits/ 8
consistently, which inevitably someone will get wrong.

+static int arcxcnn_probe(struct i2c_client *cl, const struct i2c_device_id *id)
+{
+	struct arcxcnn *lp;
+	int ret;
+	u32 i;
+	u8 regval;
+	u16 chipid;
+
+	if (!i2c_check_functionality(cl->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -EIO;
+
+	lp = devm_kzalloc(&cl->dev, sizeof(struct arcxcnn), GFP_KERNEL);

Nit: sizeof(*lp) is the preferred style.

+static const struct of_device_id arcxcnn_dt_ids[] = {
+	{ .compatible = "arc,arcxcnn", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, arcxcnn_dt_ids);

Thanks,
Mark.